fix(ai): make _context.session_id the single source of truth for the chat conversation id#1272
Merged
Merged
Conversation
…ores The remote (web) AI channel rebuilds a conversation from its `_type:"ai"` Mongo docs, and restore_history_from_db + the RemoteBackend poll both scope by `_context.session_id`. Tool-action docs (shell/task/follow_up) get that key via `_get_result_context`, but the `prompt`/`response` turns are yielded directly and only inherit `_context` from `self.context` (Runner._process_item copies `self.context` onto every item). `self.session_id` was resolved into a local var but never written back to `self.context`, so when it falls back to a locally-derived value (session_name / str(self.id)) the transcript turns land in Mongo WITHOUT `_context.session_id` — unqueryable, so a resume restores an empty history. On the platform this happened to work only because the dispatcher supplies session_id in the context. Fix the root cause: write the resolved `session_id` back onto `self.context` in `_init_options`, making it the single source of truth. Every persisted item (including prompt/response) is then stamped uniformly; idempotent on the platform where the value already matches. Also fix two stale tests that asserted the pre-`_context` query shape (top-level `session_id`) — they were failing on the branch — and add TestSessionIdStampedOnContext covering the local-fallback, platform-supplied, and restore-query-key cases. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NNjPggRSVZ2xnLb7ZxWP5H
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…ntext.session_id Now that session_id is written onto the runner context (previous commit) and therefore auto-stamped into `_context.session_id` on every persisted `_type:"ai"` doc, the top-level `Ai.session_id` field is redundant. It was write-only in core (nothing read it) and duplicated the conversation id that restore_history_from_db, the RemoteBackend poll, `_expire_stale_pending`, and secator-api all already key on via `_context.session_id`. Remove the field from the Ai output type and drop the three constructor writes (resume prompt, turn_completed marker, remote pending prompt). The conversation id is now carried in exactly one place. ActionContext.session_id and the build_pending_prompt/ask_user params are the working id and are unchanged. Cross-repo: secator-api's answer_ai_prompt + steer_doc are updated in lockstep (feat/ai-chat) to query/stamp `_context.session_id` instead of the top-level field; the UI already correlates on `_context.session_id`. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NNjPggRSVZ2xnLb7ZxWP5H
ocervell
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two commits, one theme:
_context.session_idbecomes the single source of truth for the remote AI chat conversation id.1.
fix(ai): stamp session_id on runner context so remote transcript restoresThe remote (web) AI channel is headless — a respawned
aitask rebuilds the conversation from its_type:"ai"Mongo docs, andrestore_history_from_db+ theRemoteBackendpoll both scope by_context.session_id. Tool-action docs get that key via_get_result_context, but theprompt/responseturns only inherit_contextfromself.context(Runner._process_itemcopies it).self.session_idwas resolved into a local var but never written back toself.context, so a locally-derived id left the transcript turns unqueryable → resume restored an empty history. It only worked on the platform because the dispatcher suppliessession_idin the context.Fix: write the resolved
session_idback ontoself.contextin_init_options— one line, stamps every persisted item uniformly, idempotent on the platform. Also fixes two pre-existing stale tests (asserted the old top-level query shape) and addsTestSessionIdStampedOnContext.2.
refactor(ai): retire top-level Ai.session_id fieldWith #1 guaranteeing
_context.session_idon every doc, the top-levelAi.session_idfield is redundant — it was write-only in core and duplicated the id thatrestore_history_from_db, the poll,_expire_stale_pending,poll_steers, and secator-api all already key on via_context.session_id. Removes the field + its 3 constructor writes (resume prompt, turn_completed, remote pending prompt).ActionContext.session_idand thebuild_pending_prompt/ask_userparams (the working id) are unchanged.Cross-repo (lockstep)
feat/ai-chat(PR feat(hooks) - add runtime hooks #199) — updated soanswer_ai_promptresolves the pending prompt by_context.session_id(was top-level) and the steer doc stops writing a redundant top-levelsession_id. Must merge no later than this.feat/ai-chat— no change;getChatTranscriptalready queries_context.session_id.Verification
shfmt/safecmd-parser env failures common to all branches).prompt/response/follow_updocs all carry_context.session_idand zero top-levelsession_id; answering via_context.session_id(the way updated secator-api does) is picked up by the poll and continues the turn.Part of the AI-task reliability series →
ai-resiliency(#1241).🤖 Generated with Claude Code